Skip to content

feat(coverage): warn when bundled coverage tool has no wheel for requested python_version/platform#3766

Open
Syndic wants to merge 3 commits into
bazel-contrib:mainfrom
Syndic:coverage-warn-when-missing
Open

feat(coverage): warn when bundled coverage tool has no wheel for requested python_version/platform#3766
Syndic wants to merge 3 commits into
bazel-contrib:mainfrom
Syndic:coverage-warn-when-missing

Conversation

@Syndic
Copy link
Copy Markdown
Contributor

@Syndic Syndic commented May 10, 2026

Addresses review feedback on #3764:

NOTE: Starlark unit tests have no way to capture stdout/stderr, so we can't directly assert that the warning is printed. I am not sure if the refactoring-for-testability I have considered would be appreciated, so this PR is a minimal approach. (I will also prepare an alternative PR that includes a small refactor and some tests, in case that is preferred.)

Previously, when configure_coverage_tool = True was set but the bundled coverage.py wheel set had no entry for the requested (python_version, platform), coverage_dep returned None silently. The result was that bazel coverage produced empty per-test lcov files for py_test targets with no signal to the user that coverage was unconfigured.

Print a WARNING in that path so the misconfiguration is visible. Preserves the existing silent return for the windows branch, which is intentionally quiet because the upstream coverage wrapper does not support windows.

…ested python_version/platform

Previously, when `configure_coverage_tool = True` was set but the bundled
`coverage.py` wheel set had no entry for the requested (python_version,
platform), `coverage_dep` returned None silently. The result was that
`bazel coverage` produced empty per-test lcov files for `py_test` targets
with no signal to the user that coverage was unconfigured.

Print a WARNING in that path so the misconfiguration is visible. Preserve
the existing silent return for the windows branch, which is intentionally
quiet because the upstream coverage wrapper does not support windows.
@Syndic Syndic requested review from aignas and rickeylev as code owners May 10, 2026 18:12
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates coverage_deps.bzl to issue a warning when a bundled coverage wheel is unavailable for a specific Python version and platform, replacing the previous silent failure. The warning includes instructions for manual configuration or version pinning to resolve the issue. The CHANGELOG.md was also updated to reflect this change. I have no feedback to provide as there were no review comments.

Comment thread python/private/coverage_deps.bzl Outdated
Per review on bazel-contrib#3766, replace the raw `print(...)` warning with
`logger.warn(...)` from `repo_utils.logger`, with the logger threaded
in from the caller.

- `coverage_dep` takes an optional `logger` parameter and falls back to
  a default-constructed logger if none is supplied.
- `python_register_toolchains` accepts a private `_internal_module_ctx`
  kwarg (mirroring the existing `_internal_bzlmod_toolchain_call`
  pattern). When invoked from the bzlmod path, it builds the logger
  with the real `module_ctx` so module-root filtering applies (see
  bazel-contrib#3760). For the
  WORKSPACE/macro path it constructs a minimal stand-in struct, which
  is all the logger needs.
- `python.bzl` passes `module_ctx` through.

Adds tests/coverage_deps/ with two cases (unsupported version warns,
windows stays silent) using the captured-printer pattern already
established in tests/pypi/hub_builder/hub_builder_tests.bzl.
Copy link
Copy Markdown
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! The general direction is good, when I have time I'll look at merging this.

Add a comment explaining that the supported-wheel path of coverage_dep
calls maybe(http_archive, ...) which calls native.existing_rule(),
which is only valid during BUILD/macro/finalizer evaluation, not during
rule analysis where rules_testing analysis tests run. The path is
covered end-to-end by real bazel coverage runs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants